Skip to content

Fix script output dropping last line without trailing newline#4995

Open
denik wants to merge 15 commits into
mainfrom
denik/random-bugfixes-4
Open

Fix script output dropping last line without trailing newline#4995
denik wants to merge 15 commits into
mainfrom
denik/random-bugfixes-4

Conversation

@denik
Copy link
Copy Markdown
Contributor

@denik denik commented Apr 16, 2026

Changes

Fix experimental.scripts hooks silently dropping the last line of output when it doesn't end with a trailing newline (e.g. printf "done").

Tests

Added acceptance test at acceptance/bundle/scripts/no-trailing-newline/ and unit tests for the script executor.

Running the acceptance test against the previous release demonstrates the bug:

$ go test ./acceptance -run 'TestAccept/bundle/scripts/no-trailing-newline' -useversion 0.296.0

--- Expected
+++ Actual (v0.296.0)
@@ -5,3 +5,2 @@
 line 2
-line without newline
 Name: scripts_no_trailing_newline

v0.296.0 drops "line without newline" because the last line has no trailing \n. The fix correctly includes it.

denik added a commit that referenced this pull request Apr 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 16, 2026

Approval status: pending

/acceptance/bundle/ - needs approval

5 files changed
Suggested: @pietern
Also eligible: @andrewnester, @shreyas-goenka, @janniklasrose, @lennartkats-db, @anton-107

/bundle/ - needs approval

Files: bundle/scripts/scripts.go, bundle/scripts/scripts_test.go
Suggested: @pietern
Also eligible: @andrewnester, @shreyas-goenka, @janniklasrose, @lennartkats-db, @anton-107

General files (require maintainer)

Files: NEXT_CHANGELOG.md
Based on git history:

  • @simonfaltum -- recent work in ./

Any maintainer (@andrewnester, @anton-107, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.
See OWNERS for ownership rules.

denik added a commit that referenced this pull request Apr 22, 2026
@denik denik force-pushed the denik/random-bugfixes-4 branch from a86395c to f43f4d2 Compare April 22, 2026 16:10
denik added 15 commits April 29, 2026 09:35
bufio.Reader.ReadString returns both data and io.EOF when the stream
ends without a delimiter. The old loop condition `for err == nil` caused
the last line of output to be silently dropped when it lacked a trailing
newline. This affected any bundle script (experimental.scripts hooks)
whose output didn't end with `\n`.

Restructure the loop to always process data before checking for errors,
which is the idiomatic Go pattern for ReadString.

Task: 001.md

Co-authored-by: Isaac
Co-authored-by: Isaac
This test demonstrates the bug where the last line of script output
was dropped when it didn't end with a trailing newline. Running this
test with -useversion 0.296.0 shows the bug: "line without newline"
is missing from the output.

Task: 002.md

Co-authored-by: Isaac
Regenerate outputs to reflect consolidated request logging into
out.requests.txt (replacing per-operation JSON files), updated
terraform plan snapshots, and revised test timeout behavior.

Co-authored-by: Isaac
This reverts commit 546b7a0ee. That regen was done with a 60s test
timeout that killed several long-running tests partway, leaving:

- output.txt files containing "Test script killed due to a timeout (1m0s)"
- stale out.requests.txt files that the scripts would have removed at the end
- artifacts in serverless/output/ that the script's cleanup never reached

These bad outputs broke downstream comparison tests
(default-python/classic, default-python/serverless-customcatalog) and
caused the post-test-update untracked-file check to flag the new
out.requests.txt files. The scripts on main are unchanged, so reverting
restores the correct expected outputs.

Task: 006.md

Co-authored-by: Isaac
Drop the placeholder Bundles changelog line that was never replaced when
the PR-linked entry was added, leaving a single entry referencing #4995.
Also remove TestExecuteNoScript which only re-exercises the trivial
getCommand early-return without adding coverage for the fix.

Task: 007.md
Task-review: /home/denis.bilenko/work/prompts-features/cli/random-bugfixes-4/006.SUMMARY.md

Co-authored-by: Isaac
Consolidates per-operation request recording files (out.*.requests.terraform.json,
out.post/patch.requests.txt, etc.) into unified out.requests.txt files, and
updates output.txt snapshots accordingly.

Co-authored-by: Isaac
…pelines

The previous regeneration in f41abe247 captured timed-out test runs as if
they were the correct expected output. The committed output.txt files
ended in "Test script killed due to a timeout (1m0s)", and the partial
out.requests.txt artifacts that the test scripts otherwise consume via
print_requests.py were left checked in.

Restore output.txt and out.deploy.requests*.json from before the bad regen,
and remove out.requests.txt — the scripts intentionally drop that file
on a clean run. The three tests now pass locally in under 10s each.

Task: 008.md

Co-authored-by: Isaac
Regenerate all acceptance test output files: replace generic
out.requests.txt with more specific named output files, update
output.txt files including tests that previously timed out.

Co-authored-by: Isaac
Tests that previously timed out now complete successfully. Remove
stale out.requests.txt files and add new out.requests.restore output.

Co-authored-by: Isaac
When a test script is killed by the timeout, the test directory holds
partial state: any "out.*" files reflect an incomplete recording and
output.txt contains only the prefix that streamed before the kill.
Previously `make test-update` would overwrite committed reference files
with this partial state and emit untracked "out.requests.txt" files
across cluster, deploy, and template tests.

Wrap the timeout error with the errScriptTimedOut sentinel and gate
OverwriteMode on errors.Is matching it. Also short-circuit the
unexpected-files loop so leftover partial files are not flagged or
written. Existing comparisons still run, so selftest/timeout keeps
passing.

Task: 009.md

Co-authored-by: Isaac
PR #5104 (80f7a54) renamed the secret_scopes/basic per-step files from
*.json to *.txt when migrating to gron.py --sort-arrays. The regen commit
4d38526cc on this branch accidentally resurrected the old .json file with
stale content. The script writes .txt, so the .json is unused and
test-update flags it as an unexpected output.

Task: 010.md

Co-authored-by: Isaac
@denik denik force-pushed the denik/random-bugfixes-4 branch from f43f4d2 to 7802668 Compare April 29, 2026 09:50
@denik denik temporarily deployed to test-trigger-is April 29, 2026 09:50 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is April 29, 2026 09:50 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant